Skip to content

Monitor enroll post unlock - #580

Draft
yjian118 wants to merge 5 commits into
Wind-River:masterfrom
yjian118:monitor-enroll-post-unlock
Draft

yjian118 wants to merge 5 commits into
Wind-River:masterfrom
yjian118:monitor-enroll-post-unlock

Conversation

@yjian118

@yjian118 yjian118 commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Yuxing Jiang <yuxing.jiang@windriver.com>
When sysinv-api rejected a request (host unlock, route creation, host
attribute update, ...) the Deployment Manager logged the whole wrapped
gophercloud error at error level: the escaped JSON response body plus a
stack trace. The actual reason, the sysinv faultstring, was buried in
it.

Handle every rejected sysinv-api request the same way and log only
the operation, the HTTP status and the faultstring:

- Add common/apierrors.go to extract the status code and faultstring
  from any gophercloud response error, including wrapped ones. Remote
  Python tracebacks that sysinv-api appends to conductor RPC errors
  are stripped.
- Replace ErrUnlockError with a generic ErrSystemAPI error that
  carries the operation, status code and reason. The retry delay is
  derived from the status code (408/429 transient, 5xx server error,
  other 4xx user error) and can be overridden with WithRetry. Host
  unlock rejections keep a short transient retry.
- Treat sysinv 409 Conflict (RouteAlreadyExists,
  RouteGatewayNotReachable, ...) as a rejected request, and handle 408,
  429 and other non-2xx codes that previously fell through as
  unhandled errors.
- Log rejected requests at info level with reason/status/operation
  fields so they carry no stack trace with stacktrace: error. Genuine
  errors still log at error level with a stack trace.
- Route Reconcile errors that were returned straight to
  controller-runtime through HandleReconcilerError, so they are
  classified and logged once instead of twice.
- Log sysinv request failures in the host platform network and
  manager code through the same helper.

Test Plan:
- PASS: make build, go vet, golangci-lint
- PASS: go test ./... (envtest controller suites), including new unit
        tests for sysinv error bodies, every gophercloud ErrDefaultXXX
        type, the retry policy and the handler log levels
- PASS: Reproduced a rejected host attribute update (HTTP 400) through
        the error handler with --zap-stacktrace-level=error: one INFO
        line with the faultstring, no response body or stack trace

Sample log:
2026-10-30T19:37:05.614983746Z stderr F 2026-10-30T19:37:05.614927588Z  INFO    controller.host request rejected by system API  {"request": {"name":"controller-0","namespace":"deployment"}, "operation": "failed to update memory: 942b83ed-167a-465f-85fd-fac2057f8954, {\"function\":\"vswitch\",\"vswitch_hugepages_reqd\":2,\"vswitch_hugepages_size_mib\":1024}", "status": 400, "reason": "Processor 0: vSwitch huge pages must be 0 when vSwitch type is none"}

Assisted-by: Kiro(auto model)
Signed-off-by: Yuxing Jiang <yuxing.jiang@windriver.com>
When sysinv rejects the unlock the Deployment Manager sends after
applying the deployment configuration, the Deployment Manager retries
and logs the reason only in its pod log. The playbook just waited for
the reboot and failed after dm_unlock_timeout with nothing to explain
why the host never unlocked.

Collect the unlock progress in the background while the reboot task
blocks, and report it once the wait is over:
- Add files/dm-unlock-monitor.sh, started detached through async. It
  reads only the Deployment Manager pod log, after
  dm_unlock_monitor_delay, every dm_unlock_monitor_interval. It
  records the distinct sysinv rejection reasons, matching both the
  current "request rejected by system API" format and the legacy
  "Failed to unlock" one. If no rejection was logged, it records the
  last errors instead. A line is written only when the status
  changes.
- Wrap the reboot wait in block/rescue/always. Stop the collector
  through its pid file, show the collected diagnostics, and fail with
  them if the host did not reboot. On success, clean up the scripts
  and the log. On failure, keep the log on the host.
- Add tasks/prepare.yaml to stage the fake shutdown and the monitor
  in a tempfile directory instead of /tmp/. Keep the diagnostics log
  in the ansible user's home so it survives the reboot wiping /tmp.
- Run prepare before apply-config in the initial-config (subcloud)
  and enroll paths. The Deployment Manager can unlock the host as
  soon as the configuration is applied, and copying to a host that
  is going down fails. main.yaml still runs prepare itself if it was
  not staged earlier.
- Move dm_unlock_timeout to defaults/main.yaml and add the monitor
  tunables there.

Test Plan:
- PASS: <subcloud initial-config, unlock accepted: host reboots, no
        diagnostics shown, scripts and log removed>
- PASS: <unlock rejected until timeout: playbook fails and lists the
        sysinv rejection reasons, log kept in the home directory>
- PASS: <enroll re-configuration path waits for unlock as before>

Assisted-by: Kiro(auto model)
Signed-off-by: Yuxing Jiang <yuxing.jiang@windriver.com>
The previous change parsed the sysinv faultstring out of the response
body in common/apierrors.go, reflecting over gophercloud's concrete
ErrUnexpectedResponseCode and ErrDefaultXXX types to reach the raw body.
That body parsing now lives in gophercloud itself (Wind-River/gophercloud
PR Wind-River#58): the StarlingX client returns a starlingx.FaultError whose Error()
is already the server reason with any remote Python traceback stripped.

Consume that error instead of re-deriving the reason here:

- Bump the gophercloud replace to the Wind-River starlingx branch tip
  (v0.0.0-20261105201050-704e1b14ab4f), which carries the FaultError
  and its default error context.
- Install starlingx.FaultErrorContext as the DefaultErrorContext on the
  platform client in manager/client.go, so every rejected (non 2xx)
  request from the inventory and nfv clients returns a FaultError.
- Rewrite common/apierrors.go to match the gophercloud FaultError through
  a small interface with errors.As, dropping the reflection, the body
  JSON parsing, the faultstring regex and the traceback stripping.
  APIErrorReason now just returns the error string.
- Match starlingx.FaultError in the reconciler ErrorHandler instead of
  listing each ErrDefaultXXX type, keeping ErrUnexpectedResponseCode as a
  fallback for requests that bypass the StarlingX client.
- Replace the remaining err.(gophercloud.ErrDefaultXXX) type assertions
  in the datanetwork, host and ptp controllers with APIErrorStatusCode
  checks against net/http status constants.
- Move the API-error-aware logging out of common.LogAPIError into a new
  logger package that wraps logr.Logger, so call sites keep using
  log.Error unchanged. Add logger/ to the Dockerfile build context.
- Ignore *.log files in .gitignore.

Test Plan:
- AIOSX initial deployment.
- AIOSX day-2 operation succssful.
- AIOSX day-2 memory update rejected, error message without trace stack.
- DC deployment with DX subcloud.
- AIOSX factory install.
- AIOSX subcloud enrollment.
- AIODX subcloud enrollment.
- AIOSX subcloud enrollment with double reboot by changing hugepages.
- AIOSX subcloud enrollment with mutiple new SR-iov interfaces configured
  that delays unlock process.

Assisted-by: Kiro(auto model)
Signed-off-by: Yuxing Jiang <yuxing.jiang@windriver.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant